-
Notifications
You must be signed in to change notification settings - Fork 102
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use Slevomat CS #183
base: master
Are you sure you want to change the base?
Use Slevomat CS #183
Conversation
to me it makes no sense to incorporate parts of another coding standard (other than the default sniffs provided by codesniffer) into this one. the whole idea is to provide a library with a custom set of rules based on a described coding standard. by referencing another library into this one you create an unnecessary dependency while someone can can do that customisation in their own |
The advantage IMHO is pulling in widely used, known well maintained 3rd party code (providing also auto-fixes) and thus reducing the code in this repository that needs to get maintained. For #144 (comment) the advantage would also be that no code at all in this repo would need to get written. |
by that sentiment it would make more sense to put out a seperate package which consists of a |
Something like https://github.com/mayflower/mo4-coding-standard? ;) On a more serious note, I'd say for an end-user the purpose of |
an added dependency which supports / replaces something which does not concern the core functionallity of a library makes sense. replacing the core functionality with the core functionality of another package doesn't if you'd ask me. the end user has plenty of control and can choose to use this library, another one or can aggregate a ruleset of their own liking, we don't have to make that decision. i think dropping sniffs and replacing them with a dependency is not the way to go. @djoos what's your opinion on this? |
Apologies for the late reply here... Trying to catch up here - immediately flagging that I'm in 2 minds ("well, that is not very helpful!") and definitely open for further discussion. From a coding standard point of view From a code point of view Thoughts? |
Since a few years, I personally do see that in the Slevomat case not as true anymore. Almost every sniff is highly configurable for most of them can also get configured to behave as exact opposite of the default. Thus I'm seeing it more as a collection of sniffs that can get included easily in other standards and configured there as needed, see Furthermore I also do want to say that PHPCS development itself is sometimes not turning out the way, sniff/standard authors would like to see, which is why https://phpcsutils.com/ exists and may also see more widespread use in the future (it would for sure already have, if Slevomat wouldn't have gained such popularity in the community), so limiting the source of standards, sniffs or sniff helper methods to phpcs itself brings further maintenance load in any case.
That's a weak point currently, for the Sniffs from PHPCS, already included now, but also for the combination of everything in the ruleset. For MO4, we do some light testing, but nothing regarding violations or auto-fixes. |
@wickedOne: you complete the top 3 of contributers to this repo, so I'd love to hear from you as well on this. |
first of all sincere apologies for my ridiculous late reply on this one; i'm a bit torn about this one: i'm with @djoos regarding his point breaking loose from escape studios that basically a library shouldn't have it's core feature rely an a third party library. on the other hand i'm also pro "not re-inventing the wheel" and indeed we should test the hell out of it and revert to our own implementation once things start falling apart if we decide to switch to slevomat's sniffs. given the later scenario, this repo would be nothing more than a configured phpcs.xml and a shitload of tests and offer no more added value than a config for those not willing or able to doing that themselves. my gut feeling is "stick to our own". switching to slevomat's sniffs seems premature (it's not that there are a bunch of use cases in the issue list we can't cover / test in this repository). depending on a third party for this library's core functionality defeats it's purpose and adds an undesired dependency from my point of view |
No description provided.